-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Setup more robust @replayio/playwright
tests
#542
base: main
Are you sure you want to change the base?
Setup more robust @replayio/playwright
tests
#542
Conversation
|
🚨 Potential security issues detected. Learn more about Socket for GitHub ↗︎ To accept the risk, merge this PR and you will not be notified again.
Next stepsWhat is an install script?Install scripts are run when the package is installed. The majority of malware in npm is hidden in install scripts. Packages should not be running non-essential scripts during install and there are often solutions to problems people solve with install scripts that can be run at publish time instead. What is telemetry?This package contains telemetry which tracks how it is used. Most telemetry comes with settings to disable it. Consider disabling telemetry if you do not want to be tracked. Take a deeper look at the dependencyTake a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support [AT] socket [DOT] dev. Remove the packageIf you happen to install a dependency that Socket reports as Known Malware you should immediately remove it and select a different dependency. For other alert types, you may may wish to investigate alternative packages or consider if there are other ways to mitigate the specific risk posed by the dependency. Mark a package as acceptable riskTo ignore an alert, reply with a comment starting with
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I probably should simplify this example page - it's just taken straight from create-next-app
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the core of the mocking logic. We can continue to refine this further. Right now, the test doesn't have control over WS "responses" (nor over regular requests). So It can't, for example, error any of those.
I'm looking for inspiration right now in Cypress, MSW, and Playwright. I think we could establish some conventions for aliases (like @recording[1]
and more). We could set up a queue of "route" mocks where each of the mocks would mock the respective request once. I'd really like to find a way to avoid those be sensitive to the order and timing.
} from "@replayio/protocol"; | ||
import { WebSocket } from "undici"; | ||
|
||
async function globalSetup(_config: FullConfig) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it will be best to pass a function through this config, any given test could pass a testing function in .use
within its own playwright.config.mts
and we could call this function from here with the appropriate context object.
This way each test could keep its test and mocking body close to itself (colocation, yay!).
This might require us to stringify the said function but I think it's fine. It's a standard constraint when dealing with page.evaluate
and similar APIs as those pass "functions" (in a form of string) across runtime boundaries
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This reads the list of Next.js routes to create a dynamic list of tests. In each page directory, we can create a playwright config, a playwright test, a custom page, and whatever else we might need to execute that test. Each route defined there should be a self-contained "unit". Each one of them gets executed as a separate playwright run (in an e2e~ fashion).
I'd likely have to introduce support for some pw-test-config.js
or similar. We might want to know here some things about any particular test case, such as its expected status.
I'm also thinking about passing some information from a test to this runner here - like maybe some failed assertions? Jest could create better test failure displays based on that than what we can currently get if we'd just throw within failing tests
@@ -1,7 +1,6 @@ | |||
import dbg from "./debug"; | |||
import WebSocket from "ws"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to revert this change. It's nice that it's more standard-compliant and it will be used in node itself at some point... but currently it creates warning about WebSocket
being experimental and we can't suppress them 😢
This is out of our control:
https://github.com/nodejs/node/blob/9e535b609fbb2d725465b731258d8458d14add44/lib/internal/process/warning.js#L97-L108
This PR introduces jest tests for the
@replayio/playwright
package Those run separate Playwright instances for all tests and use network mocking to do that.This way we'll be able to create new tests more easily and I hope we'll end up with good control over their executions and assertions. We should also be able to run tests in a watch mode which should make the development much easier